-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
consentmanager migration #5152
consentmanager migration #5152
Conversation
✅ Deploy Preview for prebid-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bretg everything works now.
The cookie policy page will be updated once I have added the missing vendors. I would create an account at consentmanager for you as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice progress. might need to spend some time with you to understand this new CMP interaction @muuki88
<script async src="//cdn.jsdelivr.net/npm/prebid.js@latest/dist/not-for-prod/prebid.js"></script> | ||
<script type="text/javascript" src="//services.brid.tv/player/build/brid.min.js"></script> | ||
|
||
<script type="text/plain" class="cmplazyload" data-cmp-purpose="c51" src="//services.brid.tv/player/build/brid.min.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is there a call to the Brid player in this generic-looking include file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@muuki88 - why is this generic page loading Brid?
Co-authored-by: bretg <[email protected]>
https://docs.prebid.org/prebid-video/video-integrating-solo.html keep the linked ones here and remove the rest. |
@muuki88 - looked things over. The remaining questions are about the video examples. Neither of these work for me locally
This shows the player regardless of cookie settings
This page is titled oddly and doesn’t make sense where it is in the nav bar
The Vimeo videos aren’t affected. Not following _includes/vimeo-iframe.html |
Thanks @bretg for taking a look. most of the examples did not work for me neither, but as far as I could tell, consent was not the issue. Regarding the naming, I'll take a look when I'm back from London. |
Some of these problems (e.g. the odd title) exist in the current site, so we can fix that in a separate PR. The video examples like https://docs.prebid.org/examples/video/instream/jwplayer/pb-ve-jwplayer-platform.html don't work currently either. Maybe we should just merge this PR and work with the video team to repair the examples -- maybe refactoring them to not require actual bids at the same time? |
What I can see is not working is the JW player that is missing a licence key. From a technical perspective the pipes work 😬
Yes. I'm all for that. I would phrase it that the examples are less broken, because the consent stuff at least works now. |
And it should, because it's a native HTML element 🎉 <video
id="vid1"
class="video-js vjs-default-skin vjs-big-play-centered"
controls
data-setup="{}"
width="640"
height="480"
>
<source src="https://vjs.zencdn.net/v/oceans.mp4" type="video/mp4" />
<source src="https://vjs.zencdn.net/v/oceans.webm" type="video/webm" />
<source src="https://vjs.zencdn.net/v/oceans.ogv" type="video/ogg" />
</video> There's no additional player loaded and no ads if you don't load it. I could however hide it behind consent to make it clearer. |
The naming is indeed a bit odd. The place in the sidebar however is correct. I'll open another issue to tackle this. Changing the URL structure is IMHO out of scope here.
I don't quite get this 😅 Is this a question, request or just a statement? |
@bretg . It is done 🎉 . All vimeo videos are now also behind the consent check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
<script async src="//cdn.jsdelivr.net/npm/prebid.js@latest/dist/not-for-prod/prebid.js"></script> | ||
<script type="text/javascript" src="//services.brid.tv/player/build/brid.min.js"></script> | ||
|
||
<script type="text/plain" class="cmplazyload" data-cmp-purpose="c51" src="//services.brid.tv/player/build/brid.min.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@muuki88 - why is this generic page loading Brid?
merging |
Migration from OneTrust to consentmanager.net for cookie consent.
#5149